feat(data-pipeline)!: OTLP gRPC trace export - #2171
Open
bm1549 wants to merge 7 commits into
Open
Conversation
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: a6ae5e4 | Docs | View more details | Give us feedback! |
Contributor
📚 Documentation Check Results📦
|
Contributor
🔒 Cargo Deny Results📦
|
bm1549
added a commit
that referenced
this pull request
Jun 27, 2026
The in-process exporter<->h2-server round trip is too timing-sensitive for heavily contended CI runners: the client `send` has hit its request timeout on macos-15 (GitHub) and the alpine/arm release-build matrix (GitLab) across multiple runs, despite passing locally (20/20) and on less-loaded runners, and despite the live backend verification in PR #2171 succeeding. Mark it #[ignore] so it no longer runs in the default CI suites (it can't flake the PR or burn release-build runner time), while keeping it runnable on demand via `--run-ignored all`. The gRPC export path stays covered by this crate's unit tests (ProstCodec, grpc_status_to_error, attach_metadata, build_grpc_channel, builder dispatch, protocol parse) and by the live backend verification. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dcddce271
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bm1549
added a commit
that referenced
this pull request
Jul 10, 2026
The in-process exporter<->h2-server round trip is too timing-sensitive for heavily contended CI runners: the client `send` has hit its request timeout on macos-15 (GitHub) and the alpine/arm release-build matrix (GitLab) across multiple runs, despite passing locally (20/20) and on less-loaded runners, and despite the live backend verification in PR #2171 succeeding. Mark it #[ignore] so it no longer runs in the default CI suites (it can't flake the PR or burn release-build runner time), while keeping it runnable on demand via `--run-ignored all`. The gRPC export path stays covered by this crate's unit tests (ProstCodec, grpc_status_to_error, attach_metadata, build_grpc_channel, builder dispatch, protocol parse) and by the live backend verification. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
bm1549
force-pushed
the
brian.marks/otlp-grpc-trace-export
branch
from
July 10, 2026 22:35
9dcddce to
093ed21
Compare
BenchmarksComparisonBenchmark execution time: 2026-09-03 06:25:20 Comparing candidate commit a6ae5e4 in PR branch Found 8 performance improvements and 4 performance regressions! Performance is the same for 129 metrics, 0 unstable metrics.
|
yannham
reviewed
Jul 23, 2026
Contributor
There was a problem hiding this comment.
First, apologies for waiting so long before giving you a first review. We'll avoid that kind of delay in the future.
As far as the PR goes, I have left a bunch of comments but there's no big technical blocker on my side. However I can't approve right away because the size impact is quite important. We don't have binary size quality gates (yet), but downstream consumers of libdatadog (tracers) do, which can lead to an unfortunate situation where this would get merged but tracers would be unable to update to future releases.
A first naive question: could/should this be feature-gated? I assume you'd want all tracers to support otlp out of the box, so the answers might be no, bu asking just in case.
A good test for the impact of the binary size increase: would you mind trying to create a PR against dd-trace-py pointing to this PR's libdatadog ref, and see if it triggers the size gates for e.g. serverless?
| #[cfg(test)] | ||
| mod grpc_export_tests { | ||
| use bytes::Bytes; | ||
| use h2::server; |
Contributor
There was a problem hiding this comment.
Any reason for using a h2 server instead of the usual http mock server we've been using elsewhere in tests?
Contributor
Author
There was a problem hiding this comment.
httpmock here is HTTP/1.1-only (no http2 feature) and has no trailer support, so it can't serve gRPC - the unary status rides in HTTP/2 trailers. That's why the test hand-rolls an h2 server.
Contributor
There was a problem hiding this comment.
In fact httpmock does have an "http2" features, but it's just not advertised for some reason. It enables the corresponding features on hyper. I think in the transport PR you do some tests that might be hard to reproduce with the simplest API of httpmock, but it might be worth looking at.
Contributor
Author
There was a problem hiding this comment.
Just to follow up here - httpmock has an undocumented http2 feature, but it can’t return HTTP/2 trailers. Since this test needs a trailing grpc-status: 0, it still needs the small h2 server.
Contributor
|
So, we've had a discussion with @VianneyRuhlmann about the whole worker situation. We believe we can side-step the "task in the background that needs to be restared" entirely by plugging in our own |
bm1549
force-pushed
the
brian.marks/otlp-grpc-trace-export
branch
from
July 24, 2026 20:44
093ed21 to
812e43b
Compare
bm1549
marked this pull request as draft
July 24, 2026 20:44
bm1549
changed the title
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
to DataDog/dd-trace-py
that referenced
this pull request
Aug 5, 2026
## Description When native OTLP trace export fails, `NativeWriter` currently reports its configured Datadog Agent intake URL even though libdatadog attempted the OTLP endpoint. This makes an OTLP protocol or connectivity error look like an Agent fallback. Use the configured OTLP trace endpoint in failure diagnostics whenever OTLP trace export is active. Agent export diagnostics keep their existing behavior. The original investigation also exposed a separate protocol mismatch: released versions send OTLP traces over HTTP/JSON, while the repro targeted the collector's gRPC port. HTTP/protobuf and gRPC support are already tracked by #18609, DataDog/libdatadog#2171, and DataDog/libdatadog#2273; this PR does not duplicate those transport changes. ## Testing - `scripts/run-tests --venv 1ef5a52 -- -- tests/tracer/test_writer.py -k 'native_writer_reports_otlp_intake_endpoint or native_writer_stores_otlp_endpoint'` (Python 3.13; 2 passed) - `scripts/lint checks` - `scripts/lint style -- ddtrace/internal/writer/writer.py tests/tracer/test_writer.py` - `scripts/lint spelling -- releasenotes/notes/fix-otlp-export-error-endpoint-6e7b7c8f756ef3a7.yaml` - `riot run reno lint` ## Risks Low. The change only affects the endpoint displayed in trace-export failure diagnostics when an OTLP endpoint is configured. It does not change routing, serialization, or retry behavior. ## Additional Notes The OTLP/DDOT reference and the original handoff document were corrected separately to record that the old port-4317 reproduction was testing an HTTP exporter against a gRPC receiver. Co-authored-by: munir.abdinur <munir.abdinur@datadoghq.com>
bm1549
force-pushed
the
brian.marks/otlp-grpc-trace-export
branch
from
September 1, 2026 18:52
812e43b to
13656d6
Compare
Wires OTLP gRPC trace export into TraceExporter on top of the fork-safe gRPC transport, selectable via OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc. OtlpProtocol gains a Grpc variant; the exporter dispatches through a new OtlpExportMode (Http | Grpc) to either the existing HTTP path or the gRPC transport (send_otlp_traces_grpc), with bounded exponential retry on transient IO matching the HTTP path. The OTLP resource info is built once at construction and shared by both paths. gRPC is native-only; wasm32 rejects it at build time. Includes a public-API end-to-end gRPC export test. BREAKING CHANGE: adds the Grpc variant to the exhaustive public OtlpProtocol enum, so exhaustive matches on it must add an arm. libdatadog consumers pin by version and pick this up on the next release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bm1549
force-pushed
the
brian.marks/otlp-grpc-trace-export
branch
from
September 1, 2026 19:17
13656d6 to
f0e93c2
Compare
bm1549
marked this pull request as ready for review
September 1, 2026 20:39
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0e93c2dce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Default portless HTTP endpoints to port 80 while rejecting ambiguous authorities. Decode google.rpc.RetryInfo locally to avoid another tonic dependency, retry recoverable resource exhaustion with capped exponential delays, and share protobuf requests across attempts.
yannham
approved these changes
Sep 2, 2026
| /// ([`Self::set_otlp_metrics_endpoint`]). | ||
| /// | ||
| /// OTLP and an agent URL may coexist: the agent URL is still useful for auxiliary | ||
| /// agent endpoints (info, stats) even when trace payloads are routed to OTLP. |
Contributor
There was a problem hiding this comment.
Any reason for removing all of this? Did of it become obsolete?
Comment on lines
+83
to
+84
| .checked_pow(attempt.saturating_sub(1)) | ||
| .unwrap_or(u32::MAX); |
Contributor
There was a problem hiding this comment.
Suggested change
| .checked_pow(attempt.saturating_sub(1)) | |
| .unwrap_or(u32::MAX); | |
| .saturating_pow(attempt.saturating_sub(1)); |
Comment on lines
+86
to
+87
| .checked_mul(multiplier) | ||
| .unwrap_or(Duration::MAX) |
Contributor
There was a problem hiding this comment.
Suggested change
| .checked_mul(multiplier) | |
| .unwrap_or(Duration::MAX) | |
| .saturating_mul(multiplier) |
| // instead of going through the `Encoder`/`Decoder` traits (see `codec_tests`). | ||
| fn encode_into(item: T, dst: &mut impl bytes::BufMut) -> Result<(), Status> { | ||
| item.encode(dst) | ||
| fn encode_into(item: Arc<T>, dst: &mut impl bytes::BufMut) -> Result<(), Status> { |
Contributor
There was a problem hiding this comment.
Out of curiosity, why do we need to switch to Arc here? Also, couldn't the encode just take a reference, leaving the choice to the caller whether to use Rc, Arc, & or whatnot
| } | ||
|
|
||
| // Insert the pre-validated custom headers, the optional test-session token, and (when enabled) the | ||
| // client-computed-stats marker into the request metadata. |
Contributor
There was a problem hiding this comment.
Same question: is this removal intentional?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds OTLP gRPC as a trace-export transport, selectable with
OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc.OtlpProtocolgains aGrpcvariant, andTraceExporterdispatches throughOtlpExportMode(HttporGrpc). The gRPC path limits response bodies to 4 MiB and reports partial-success responses. Remote-status diagnostics retain the original gRPC code. OTLP resource metadata is built once when the exporter is constructed, including the client-computed-stats marker. gRPC is native-only; wasm32 rejects it during build. The FFI acceptsgrpc, so SDKs using libdatadog can select the transport.The transport validates its dial authority before creating the exporter. Portless
http://endpoints use port 80. Valid explicit DNS and IPv6 ports are preserved; empty hosts, malformed ports, and userinfo are rejected. Retryable gRPC errors use bounded exponential backoff with jitter.RESOURCE_EXHAUSTEDis retried only when the server suppliesgoogle.rpc.RetryInfo. A positive server delay is used as the backoff base when it fits within the 30-second retry budget; longer delays stop retries instead of retrying before the server requested. Zero or missing delays use the existing 100 ms base. Protobuf requests are shared across attempts instead of deep-cloned.The fork-safe gRPC transport primitive landed in #2273. This PR integrates it with the exporter and FFI and includes the transport hardening found during review. Completed responses now win over subsequent connection teardown, and transient HTTP/2 stream resets are classified as retryable transport errors. Remote gRPC statuses keep their code and status classification instead of being reported as local I/O failures. The gRPC path sends the standard libdatadog user-agent and entity headers and emits the native trace-export telemetry now used by the other transports.
Motivation
libdatadog's OTLP trace export was HTTP-only. Java and .NET implement gRPC OTLP separately; PHP, Python, Ruby, and Rust use libdatadog for trace export. Adding gRPC here lets their integrations honor
OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpcinstead of rejecting it or falling back to another transport.Additional Notes
GrpcServiceover hyper h2c. It creates a connection per request and has no persistent worker, which keeps it safe across forks.OtlpProtocol::from_str("grpc")now succeeds. Adding a variant to the exhaustive public enum is a breaking change, as noted in the commit footer.Cancelled,Aborted,OutOfRange,DataLoss,Unavailable, andDeadlineExceeded) getOTLP_MAX_RETRIES + 1total attempts.RetryInfocontrols recoverable throttling delays; other retries use the existing 100 ms exponential base, with up to 100 ms of jitter.RequestErrorand FFI error classification, while retaining the gRPC code in the message..set_client_computed_stats()in the resource attributes and request metadata.How to test the change?
cargo check -p libdd-data-pipelinecargo +nightly-2026-07-26 fmt --all -- --checkcargo +stable clippy -p libdd-data-pipeline --all-targets --no-deps -- -D warningscargo +nightly clippy -p libdd-data-pipeline --test test_trace_exporter_otlp_grpc -- -D warningscargo +1.87.0 check -p libdd-data-pipeline --test test_trace_exporter_otlp_grpccargo nextest run -p libdd-data-pipeline -E '!test(tracing_integration_tests::)'(204 passed; 7 Docker-backed integration tests skipped locally)cargo test -p libdd-data-pipeline grpc --lib(34 passed)cargo test -p libdd-data-pipeline --test test_trace_exporter_otlp_grpc(2 passed)cargo test -p libdd-data-pipeline --doc(2 passed)cargo check -p libdd-data-pipeline --target wasm32-unknown-unknown --no-default-featurescargo ffi-test --skip-build --filter trace_exporter(1 passed; the full FFI build succeeded)cargo +stable clippy -p libdd-data-pipeline --all-targets -- -D warningscurrently stops on a pre-existingcollapsible_matchwarning inlibdd-trace-utils/src/tracer_payload.rs, which this PR does not modify.